Skip to content

Fix #19664: Static extension overload resolution for generic types#19736

Open
T-Gro wants to merge 2 commits into
mainfrom
fix/19664-extension-overloads
Open

Fix #19664: Static extension overload resolution for generic types#19736
T-Gro wants to merge 2 commits into
mainfrom
fix/19664-extension-overloads

Conversation

@T-Gro
Copy link
Copy Markdown
Member

@T-Gro T-Gro commented May 14, 2026

Fixes #19664

When resolving Type<TArg>.Member(...), the name resolver uses LookupIsInstance.Yes (dot-expression form) but finds static intrinsic members. The extension member lookup then also filters for instance-only extensions, missing the static extension overloads entirely.

The fix detects this mismatch and relaxes the filter to LookupIsInstance.Ambivalent so static extensions are included in overload resolution.

type StaticGeneric<'T> =
    static member Bar() = ()

[<AutoOpen>]
module StaticGenericExtensions =
    type StaticGeneric<'T> with
        static member Bar(_: int) = ()  // Previously: FS0505

StaticGeneric<int>.Bar(42)  // Now resolves correctly

T-Gro and others added 2 commits May 12, 2026 15:57
@T-Gro
Add failing regression test for issue #19664 (static extension overlo…
886b4a0 
…ad resolution with explicit type args)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
Fix #19664: include static extensions when resolving Type<args>.Membe…
1d64720 
…r (TDD green)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro T-Gro requested a review from a team as a code owner May 14, 2026 14:13
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 14, 2026

❗ Release notes required

✅ Found changes and release notes in following paths:

Warning

No PR link found in some release notes, please consider adding it.

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md No current pull request URL (#19736) found, please consider adding it

@T-Gro T-Gro requested a review from abonie May 14, 2026 14:15
@T-Gro T-Gro added AI-Auto-Resolve-CI Opt-in: LabelOps triages CI failures on this PR every 3h AI-Auto-Resolve-Conflicts Opt-in: LabelOps merges main into this PR and resolves conflicts every 3h labels May 14, 2026
@T-Gro T-Gro enabled auto-merge (squash) May 14, 2026 14:16
@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label May 14, 2026
@github-actions github-actions Bot mentioned this pull request May 14, 2026
Open
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 14, 2026

🔍 Tooling Safety Check — Bypassed (non-fork)

(scanned-sha)1d647200b286e43951fb9462fcf29579bc3809d9(/scanned-sha)

Generated by PR Tooling Safety Check · ● 1.9M · ◷

T-Gro
T-Gro commented May 18, 2026
View reviewed changes
Copy link
Copy Markdown
Member Author

@T-Gro T-Gro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review – well-targeted fix with clear comments, good regression test, and correct release notes. LGTM.

Summary of the change: When resolving Type<TArg>.Member(...), the resolver enters via dot-expression form (LookupIsInstance.Yes) but TryFindIntrinsicNamedItemOfType returns all matching methods regardless of instance/static. The extension method lookup then applies the strict instance-only filter, missing static extensions. The fix detects when the intrinsic set contradicts the filter and relaxes to Ambivalent, allowing static extensions to participate in overload resolution.

Minor observations (non-blocking):

  1. Ambivalent branch in the isAmbivalent check – The LookupIsInstance.Ambivalent -> true case means List.exists returns true for any non-empty minfos, then re-assigns isInstanceFilter = Ambivalent (already its value). This is harmless but reads slightly confusingly. You could simplify to only check the Yes/No cases:
    \\ sharp
    let isInstanceFilter =
    match isInstanceFilter with
    | LookupIsInstance.Ambivalent -> isInstanceFilter
    | LookupIsInstance.Yes when minfos |> List.exists (fun m -> not m.IsInstance) ->
    LookupIsInstance.Ambivalent
    | LookupIsInstance.No when minfos |> List.exists (fun m -> m.IsInstance) ->
    LookupIsInstance.Ambivalent
    | _ -> isInstanceFilter
    \
    Purely a readability suggestion – the current code is functionally correct.

  2. Property-branch symmetry – The PropertyItem arm (line ~2818) and the RFC-1137 extension-method lookup (line ~2864) both use isInstanceFilter without this relaxation. If a similar scenario can occur with static extension properties on generic types via explicit type args, the same fix may be needed there. Not required for this PR though.

  3. Release-notes entry – minor: once merged, add the PR link ([PR #19736](...)) to match the other entries in the file.

@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abonie abonie Awaiting requested review from abonie

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

AI-Auto-Resolve-CI Opt-in: LabelOps triages CI failures on this PR every 3h AI-Auto-Resolve-Conflicts Opt-in: LabelOps merges main into this PR and resolves conflicts every 3h AI-reviewed PR reviewed by AI review council AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed

Projects

F# Compiler and Tooling
Status: New

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Overload resolution of static member extensions for generic types broken

1 participant

@T-Gro